Skip to content
This repository has been archived by the owner on Oct 26, 2022. It is now read-only.

Upgrade tokio to 1.0 #134

Merged
merged 1 commit into from
Jan 20, 2021
Merged

Conversation

Tim-Zhang
Copy link
Contributor

@Tim-Zhang Tim-Zhang commented Jan 15, 2021

Fixes: #105

@little-dude
Copy link
Owner

Hey, thanks a lot @Tim-Zhang I'll try to review this week-end!

@little-dude little-dude self-requested a review January 16, 2021 10:01
@little-dude
Copy link
Owner

@gabrik you might also be interested in this if you're working on #77

Copy link
Contributor

@mcr mcr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have read through the patch set, and I can't say that I exactly understand any of it: clearly I need much more knowledge of tokio. It seems that bulk is that: Eventd trait is no more, we go from PollEvented to AsyncFd interface, and that the poll_fn() gets to make use of some refactored changes. The comments at line 66 which go away maybe could be re-interpreted to explain how we use the guard and poll_write_ready() interface so that future maintainers can understand what's going on.

@Tim-Zhang
Copy link
Contributor Author

@mcr Thank you for your review. The reason I removed the comments at line 66 is that I think the code is clear and obviously, It's a fixed form and document in docs of tokio, but I overlooked that some developers are not familiar with tokio. I will add these comments back, thanks.

Fixes: little-dude#105

Signed-off-by: Tim Zhang <tim@hyper.sh>
Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {
self.0.clear_write_ready(cx)?;
Poll::Pending
loop {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like below, I'm not sure to understand why we need to loop here.

Copy link
Owner

@little-dude little-dude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, I just have a question about the loop that was added to the poll_xxx methods. Thanks for working on this @Tim-Zhang !

@Tim-Zhang
Copy link
Contributor Author

Tim-Zhang commented Jan 18, 2021

@little-dude I think we should bump the major version after this pr merged, Because of tokio's upgrade, netlink is not compatible with before

@mxpv mxpv mentioned this pull request Jan 19, 2021
@little-dude little-dude merged commit e39693c into little-dude:master Jan 20, 2021
@little-dude
Copy link
Owner

Thanks a ton @Tim-Zhang !

@Tim-Zhang Tim-Zhang deleted the update-tokio-to-1.0 branch January 27, 2021 13:19
little-dude added a commit that referenced this pull request Nov 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate to tokio 0.3 / mio 0.7
3 participants